Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not mark visit functions as pure #381

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

tbleher
Copy link
Contributor

@tbleher tbleher commented Jun 29, 2023

Marking a function as pure implies that it does not have any observable side effects (only the return value is used). So if the compiler detects that the return value of a pure function is not used, it may eliminate the call to the pure function.

This would be wrong, however, if the passed in Visitor has side effects which are needed by the program.
Fix this by not marking the visit functions as pure.

Marking a function as pure implies that it does not have any observable
side effects (only the return value is used). So if the compiler detects
that the return value of a pure function is not used, it may eliminate the
call to the pure function.

This would be wrong, however, if the passed in Visitor has side effects
which are needed by the program.
Fix this by not marking the visit functions as pure.
@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Merging #381 (f132b6c) into master (81b2685) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   96.59%   96.58%   -0.01%     
==========================================
  Files          22       22              
  Lines        8155     8150       -5     
==========================================
- Hits         7877     7872       -5     
  Misses        278      278              
Impacted Files Coverage Δ
src/c4/yml/node.hpp 98.51% <ø> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@biojppm biojppm merged commit d1971b6 into biojppm:master Jun 30, 2023
@biojppm
Copy link
Owner

biojppm commented Jun 30, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants